Bug-1915777: Make test tags theme aware#993
Conversation
✅ Deploy Preview for mozilla-perfcompare ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #993 +/- ##
=======================================
Coverage 96.46% 96.46%
=======================================
Files 104 104
Lines 3028 3028
Branches 694 694
=======================================
Hits 2921 2921
Misses 106 106
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Converting this to draft. I will have a look at improving the tests |
|
@kala-moz Since this is a change to an UI element color, do we have to write tests for it ? |
|
Adding latest screenshots 
|
|
@kala-moz I am opening this back for review again. In the latest commit, I have moved the logic to get the required colors for tags and text into helper functions. These helper functions take the theme as argument. Unit tests have been added to validate the helpers. The changes have been rebased over the latest changes in main. |
There was a problem hiding this comment.
Thanks for your PR! There is a faster way to accomplish this using MUI's dark theme setup.
First, go to
perfcompare/src/theme/protocolTheme.ts
Line 49 in fda1fb2
And add the colors for the tag options for both dark and light mode, along with the a new inverted property for text:
const lightMode = {
...,
text: {
...,
inverted: Colors.InvertedText,
},
...,
tagOptions: {
option1: Colors.TagOptionBackground3n,
option2: Colors.TagOptionBackground3n1,
option3: Colors.TagOptionBackground3n2,
},
};
const darkMode = {
...,
text: {
...,
inverted: Colors.InvertedTextDark,
},
...,
tagOptions: {
option1: Colors.TagOptionBackground3nDark,
option2: Colors.TagOptionBackground3n1Dark,
option3: Colors.TagOptionBackground3n2Dark,
},
};
Next, add a Box MUI element to TestHeader here replacing the parent div and and add the overriding sx prop styles:
<Box
sx={{
display: 'flex',
flexWrap: 'wrap',
gap: '4px',
margin: '0 4px',
textAlign: 'right',
'& span': {
color: 'text.inverted',
},
'& span:nth-child(3n)': {
//using our new tag options color here that we created in protocolTheme
bgcolor: 'tagOptions.option1',
},
'& span:nth-child(3n+1)': {
bgcolor: 'tagOptions.option2',
},
'& span:nth-child(3n+2)': {
bgcolor: 'tagOptions.option3',
},
}}
>
....
</Box>
Do the same for SubtestRevisionHeader:
You can delete all the unused code and no need for new tests.
This change updates the TestHeader and SubtestsRevisionHeader by making tag options aware of light. The changes made are: * Import theme hook in both TestHeader and SubtestsRevisionHeader * Update `styles` object in both TestHeader and SubtestsRevisionHeader to choose background color of span based on theme * Update color of tag based on theme * Add new colors to `styles/Colors.ts`
Moved the logic to find the appropriate colors for the tags into helpers and used those helpers in the components. Also, added tests to validate the helpers.
Implemented review comment and used Mui's theme aware `<Box>` element. Added the required options to `protocolTheme.ts`. Also, removed implementation added in previous commits as well as the tests and regenerated the spanshots by running `npm run fix-all`
|
@kala-moz Thank you for guiding me on this. My apologies for the old implementation as I am new to MUI. TestHeader screenshot
SubtestsRevisionHeader screenshot
|
2 participants
This change updates the tag options used in the
TestHeaderandSubtestsRevisionHeaderto change the tag background and font based on the theme chosen (dark / light). New background colors are defined in thestyles/Colors.tsfor the option and existing font colors are reused for the font.This has been tested locally and the screenshots are as below.
npm run fix-allhas been applied before creating the commit.@davehunt @kala-moz